Skip to content

test: quote check-doc grep path#323

Open
wangedmund77-cmyk wants to merge 1 commit into
BitgesellOfficial:masterfrom
wangedmund77-cmyk:codex/fix-check-doc-space-path
Open

test: quote check-doc grep path#323
wangedmund77-cmyk wants to merge 1 commit into
BitgesellOfficial:masterfrom
wangedmund77-cmyk:codex/fix-check-doc-space-path

Conversation

@wangedmund77-cmyk
Copy link
Copy Markdown

Summary

  • quote the check-doc.py source-tree path used inside shell-based git grep commands
  • keeps the lint working when the repository checkout path contains spaces

Validation

  • python3 test/lint/check-doc.py
  • git diff --check

Refs #32.

@MyTH-zyxeon
Copy link
Copy Markdown

Review-assist for #323 under the Bitgesell improvement queue (#81):

  • Scope is narrow and useful: test/lint/check-doc.py shells out through formatted command strings, so quoting the resolved source-tree path is the right fix for checkouts stored under paths with spaces.
  • Please keep one validation example that actually runs from a path containing a space, even if only documented in the PR body, because the normal python3 test/lint/check-doc.py run from a no-space checkout does not exercise the regression.
  • The quoted value flows through three grep commands. Before merge, I would confirm the -- <path> ':(exclude)src/test/' argument order still behaves as intended on the project-supported shell environment.
  • This change intentionally quotes only the source path, not the exclude path. That is probably fine because FOLDER_TEST is repo-relative and static, but it is worth preserving that assumption in the review notes.
  • For BGL (Bitgesell) Bounty/Improvement Program ($50000 budget) #81 payout hygiene, this looks like a small CI/lint portability fix rather than a broad feature; I would count it only after maintainers confirm this class of test hardening is eligible.

I only reviewed the public diff. No duplicate implementation PR, live-chain action, secret material, dependency install, or payment action from me here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants